feat(lambda-events): add VPC Lattice event structures#1036
feat(lambda-events): add VPC Lattice event structures#1036andrewpatto wants to merge 12 commits intoaws:mainfrom
Conversation
andrewpatto
changed the title
andrewpatto
changed the title
|
If you don't mind, I will take over this PR to make adjustments based on the changes we've done over the last few months (mainly non-exhaustive). |
jlizen
left a comment
jlizen
left a comment
There was a problem hiding this comment.
Overall seems reasonable, but is it desirable to have so many of these fields output as null if unset? I would have expected a skip_serializing_if statement.
And then I had some questions about whether certain fields are really optional.
No problems. The original PR was a straight conversion of the schemas from the docs and mirroring other conventions from this library. Happy for actual experts to take over! |
- Add #[non_exhaustive] to all VPC Lattice structs - Change status_code from u16 to i64 for consistency with ALB/API Gateway - Move comma-separated header serde into custom_serde/headers.rs - Fix test imports and function naming
FullyTyped
force-pushed
the
main
branch
2 times, most recently
from
6ae324c to
ab72f5f
Compare
- Add builder support to all VPC Lattice structs - Add VpcLatticeRequest untagged enum for V1/V2 dispatch - Use u16 for response status_code - Make path, version, and request context fields non-optional - Use Body type for request body instead of String - Remove undocumented statusDescription field from response - Remove unnecessary serde(bound) on request_context
jlizen
left a comment
jlizen
left a comment
There was a problem hiding this comment.
Thanks for the cleanup! Some nits that are all probably bad ideas anyway, I'm fine with what you have.
| /// Deserialize headers, splitting comma-separated values into multiple header values. | ||
| /// Used by VPC Lattice V1 which sends multi-value headers as "value1, value2". | ||
| #[cfg(feature = "vpc_lattice")] | ||
| pub(crate) fn deserialize_comma_separated_headers<'de, D>(de: D) -> Result<HeaderMap, D::Error> |
There was a problem hiding this comment.
Oof, thanks for slogging through this one.
| pub struct VpcLatticeRequestV2 { | ||
| /// The version of the event structure (always "2.0" for V2) | ||
| #[serde(default = "default_version")] | ||
| pub version: String, |
There was a problem hiding this comment.
Minor: if it is actually always 2.0, should we consider unconditionally deserializing to a &'static str?
I guess, it could easily become 2.1 eventually, at which point we are stuck managing more complicated deserialization logic, so better to take the minor perf hit of a string?
Probably the ideal would be something like a Cow<'static, str> so that we could initially mark it with #[serde(skip)] and have custom deserialization, but we could roll forward with owned in the future.
Anyway I think it's probably ok to optimize for simplicity, just wondering out loud.
| pub region: String, | ||
|
|
||
| /// Time of the request in microseconds since epoch | ||
| pub time_epoch: String, |
There was a problem hiding this comment.
nit: I might personally prefer time_epoch_micros with a #[serde(rename = "timeEpoch")], but I can't tell if that's more confusing to deviate from upstream. Just my assumption when I see epoch time, is not micros ^^
| "user-agent": "curl/7.68.0", | ||
| "x-forwarded-for": "10.0.2.100", | ||
| "authorization": "Bearer abc123def456", | ||
| "multi": "abcd, DEF" |
| "accept": ["*/*"], | ||
| "user-agent": ["curl/7.68.0"], | ||
| "x-forwarded-for": ["10.0.2.100"], | ||
| "multi": ["x", "y"] |
|
@maxday / @mpindaru would it be possible to get a second pair of eyes from somebody on your team for correctness relative to spec? |
3 participants
📬 Issue #, if available:
#1028
✍️ Description of changes:
Added event structures for the VPC lattice service - including tests and sample request/responses.
🔏 By submitting this pull request
cargo +nightly fmt.cargo clippy --fix.